DM-46356: Update testing to use new IsrTaskLSST calibration pipeline #41
Conversation
There was a problem hiding this comment.
I think the small pipeline rearranging suggested would be helpful for the future (and to keep all pipeline constructions consistent), but that can be pushed to a separate ticket (since this is already in a mess of merges).
Same with the certify/verify command flips: I think we should do it, but a separate ticket would be fine to make sure the rest of the merge is smooth.
| [defects], | ||
| [ | ||
| getPipeTaskCmd("linearizer", exposureDict["ptcExposurePairs"], "cpLinearizer.yaml"), | ||
| getCertifyCmd("linearizer"), |
There was a problem hiding this comment.
Should the verify command run before the certify command? I see that the current code matches what's here, but when we run these for real, the verify is run against the uncertified calibration. It probably doesn't matter, but if we have connections that can only be met by certified calibrations, then that could cause problems in production. I'm happy to take any tickets this spawns if something is broken.
There was a problem hiding this comment.
I've added DM-46432 to look into this, so it doesn't need to complicate the merging here. That ticket will decide what the right solution is.
DATA/SConscript
Outdated
| inputCollections = f"ci_cpp_ptc,{inputCollections}" | ||
|
|
||
| pipelineYaml = os.path.join(PKG_ROOT, "pipelines", pipelineFile) | ||
| pipelineYaml = os.path.join(PKG_ROOT, "pipelines", f"LATISS_legacy_{legacyDate}", pipelineFile) |
There was a problem hiding this comment.
I think this should be "LATISS", f"legacy_{legacyDate}" so the ci_cpp_gen3 override pipelines match the paths of the cp_pipe and cp_verify legacy pipelines. This will require runIsrLSST.yaml to be moved into a LATISS subdirectory, but that will make extensions to different camera datasets easier in the future.
| @@ -1,7 +1,7 @@ | |||
| description: ci_cpp CROSSTALK calibration construction | |||
| instrument: lsst.obs.lsst.Latiss | |||
| imports: | |||
| - location: $CP_PIPE_DIR/pipelines/LATISS/cpCrosstalk.yaml | |||
| - location: $CP_PIPE_DIR/pipelines/LATISS/legacy_202409/cpCrosstalk.yaml | |||
There was a problem hiding this comment.
See comment above about adding subdirectories to have matching pipeline paths.
pipelines/runIsrLSST.yaml
Outdated
| class: lsst.ip.isr.IsrTaskLSST | ||
| config: | ||
| doBrighterFatter: false | ||
| doDeferredCharge: false |
There was a problem hiding this comment.
Mentioned above for instrument-specific pipeline path.
| @@ -1,16 +1,11 @@ | |||
| 2021052500015: | |||
| FAILURES: | |||
| - RXX_S00 C11 CR_NOISE | |||
| SUCCESS: true | |||
There was a problem hiding this comment.
This is reassuring that we're dealing with defects better now.
erykoff
force-pushed
the
tickets/DM-46356
branch
from
65ef716
to
b416b00
Compare
Now setting CI_CPP_LEGACY environment variable to 1 will run the "legacy" pipelines and tests.
erykoff
force-pushed
the
tickets/DM-46356
branch
from
b416b00
to
7c4374b
Compare
The BFK itself is still unverified and may not be good; see DM-46445.
The old "legacy" calibration pipelines can still be tested locally by setting the environment variable
export CI_CPP_LEGACY=1.